fix(storage): wrap remaining saveAccounts call sites with retry helper - #443

Summary

Three production paths still wrote raw await saveAccounts(storage) without the EBUSY/EPERM retry helper that every other persistence boundary in this codebase uses. Wraps them in saveAccountsWithRetry so a transient Windows file-lock contention no longer silently drops a sync (or, in switch flows, fails the whole operation).

Why this matters

Per lib/AGENTS.md ("focus on auth rotation, windows filesystem io, and concurrency. verify every change … new queues handle ebusy/429 scenarios"), every persistence boundary should be EBUSY-tolerant. The forecast / report / best / rotation-reset / new rotation-reset-rate-limits paths all already use saveAccountsWithRetry from lib/codex-manager/forecast-report-shared.ts. These three were the holdouts I found while auditing the codebase after #442:

Changes

• lib/accounts.ts — import saveAccountsWithRetry, replace one raw call.
• lib/codex-manager.ts — import saveAccountsWithRetry, replace two raw calls.
• test/accounts-edge.test.ts — new regression: loadFromDisk retries source-of-truth persist on transient EBUSY.
• test/codex-manager-cli.test.ts — existing does not mutate loaded quota cache when live check account save fails updated to use mockRejectedValue (unbounded) so the retry exhausts and the test still asserts its intended rejection path.

Retry policy (unchanged)

Same as the existing saveAccountsWithRetry helper:

• Up to 3 retries on EBUSY / EPERM with backoff (10 * 2 ** attempt ms).
• Any non-retryable error (no code, or a code outside the EBUSY/EPERM set) re-throws on the first attempt.
• The existing test loadFromDisk tolerates sync persistence failures keeps its 1-call assertion: that error has no code, so it short-circuits as before.

Test plan

• npm run typecheck
• npx eslint
• Full suite: 3745 / 3745 pass
• Targeted: new EBUSY-retry regression in accounts-edge.test.ts confirms two attempts on a single transient failure.

Notes

This is the first follow-up from a broader reliability audit of the codebase that surfaced four PR-sized improvements. Subsequent PRs will cover the path-singleton try/finally hardening, a concurrency regression test for path-singleton races, and cleanup of the unused lastAccountEmail field. Submitting them as separate PRs so each can be reviewed in isolation.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

wraps the three remaining raw saveAccounts call sites in saveAccountsWithRetry so transient windows file-lock errors (EBUSY/EPERM) no longer silently drop a startup sync or abort a switch/health-check command. all three paths now match the retry policy already in place everywhere else in the codebase.

Confidence Score: 5/5

safe to merge — only P2 style finding, logic and token safety are correct

all three call sites correctly pass the locally-imported saveAccounts as a callback, matching existing usage; no circular dependency introduced by the accounts.ts → forecast-report-shared import; retry semantics and error propagation are correct; tests cover transient recovery, exhaustion, and the no-codex-cli-sync guarantee; only finding is a cosmetic import ordering issue

no files require special attention

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Caller
    participant saveAccountsWithRetry
    participant saveAccounts (fs)

    Caller->>saveAccountsWithRetry: saveAccountsWithRetry(storage, saveAccounts)
    loop attempt 0..3
        saveAccountsWithRetry->>saveAccounts (fs): saveAccounts(storage)
        alt success
            saveAccounts (fs)-->>saveAccountsWithRetry: resolved
            saveAccountsWithRetry-->>Caller: return
        else EBUSY / EPERM and attempt < 3
            saveAccounts (fs)-->>saveAccountsWithRetry: throw {code: EBUSY|EPERM}
            saveAccountsWithRetry->>saveAccountsWithRetry: sleep(10 * 2^attempt ms)
        else non-retryable OR attempt >= 3
            saveAccounts (fs)-->>saveAccountsWithRetry: throw error
            saveAccountsWithRetry-->>Caller: rethrow
        end
    end

This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 41-42

Comment:
**import inserted mid-`commands/` block**

`forecast-report-shared.js` sorts lexically after `commands/` (`f` > `c`), so it lands in the middle of the alphabetical `commands/` import group. consider moving it after the last `commands/` import to keep the block sorted and readable.

```suggestion
import { runConfigExplainCommand } from "./codex-manager/commands/config-explain.js";
import { runDebugBundleCommand } from "./codex-manager/commands/debug-bundle.js";
import { saveAccountsWithRetry } from "./codex-manager/forecast-report-shared.js";
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "test(codex-manager): cover persistAndSyn..." | Re-trigger Greptile}

Context used:

• Context used - lib/AGENTS.md (source)